Skip to content

correcao do uso de git fetch no script#68

Closed
anaalves-asaas wants to merge 4 commits intomainfrom
ADQ-558-codenarc
Closed

correcao do uso de git fetch no script#68
anaalves-asaas wants to merge 4 commits intomainfrom
ADQ-558-codenarc

Conversation

@anaalves-asaas
Copy link
Contributor

No description provided.

@anaalves-asaas anaalves-asaas requested a review from a team February 3, 2026 14:48
@anaalves-asaas anaalves-asaas self-assigned this Feb 3, 2026
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

$includes_arg >/dev/null 2>&1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

${INPUT_REVIEWDOG_FLAGS} >/dev/null || true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

${INPUT_REVIEWDOG_FLAGS} >/dev/null || true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 [shellcheck] reported by reviewdog 🐶
Double quote to prevent globbing and word splitting. SC2086

${INPUT_REVIEWDOG_FLAGS} >/dev/null || true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[shellcheck (suggestion)] reported by reviewdog 🐶

$includes_arg >/dev/null 2>&1

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[shellcheck (suggestion)] reported by reviewdog 🐶

${INPUT_REVIEWDOG_FLAGS} >/dev/null || true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[shellcheck (suggestion)] reported by reviewdog 🐶

${INPUT_REVIEWDOG_FLAGS} >/dev/null || true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[shellcheck (suggestion)] reported by reviewdog 🐶

${INPUT_REVIEWDOG_FLAGS} >/dev/null || true

Copy link

@amazon-q-developer amazon-q-developer bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR improves the git fetch logic in the CodeNarc script by adding a check to see if commits already exist locally before attempting to fetch them. This is a good optimization that can reduce unnecessary network calls.

Key Changes:

  • ✅ Added git cat-file -e checks to verify if commits exist locally before fetching
  • ✅ Improved error output redirection consistency with 2>&1
  • ✅ Better error handling in build_changed_lines_cache function

Issues Found:

  • The git diff commands in the generate_git_diff function lack proper error handling, which could lead to silent failures when diff operations fail

The changes are functionally correct and improve the script's efficiency. Please address the error handling issues in the git diff commands to ensure robust operation.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

git fetch origin "$GITHUB_HEAD_SHA" --depth=1 >/dev/null 2>&1 || true
git diff -U0 "$GITHUB_BASE_SHA" "$GITHUB_HEAD_SHA" -- '*.groovy'
if git cat-file -e "$GITHUB_BASE_SHA" 2>/dev/null && git cat-file -e "$GITHUB_HEAD_SHA" 2>/dev/null; then
git diff -U0 "$GITHUB_BASE_SHA" "$GITHUB_HEAD_SHA" -- '*.groovy' 2>&1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The git diff command should include error handling to prevent silent failures when the diff operation fails.

Suggested change
git diff -U0 "$GITHUB_BASE_SHA" "$GITHUB_HEAD_SHA" -- '*.groovy' 2>&1
git diff -U0 "$GITHUB_BASE_SHA" "$GITHUB_HEAD_SHA" -- '*.groovy' 2>&1 || return 1

else
git fetch origin "$GITHUB_BASE_SHA" --depth=1 2>&1 || true
git fetch origin "$GITHUB_HEAD_SHA" --depth=1 2>&1 || true
git diff -U0 "$GITHUB_BASE_SHA" "$GITHUB_HEAD_SHA" -- '*.groovy' 2>&1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The git diff command should include error handling to prevent silent failures when the diff operation fails.

Suggested change
git diff -U0 "$GITHUB_BASE_SHA" "$GITHUB_HEAD_SHA" -- '*.groovy' 2>&1
git diff -U0 "$GITHUB_BASE_SHA" "$GITHUB_HEAD_SHA" -- '*.groovy' 2>&1 || return 1

fi
else
git diff -U0 HEAD~1 -- '*.groovy'
git diff -U0 HEAD~1 -- '*.groovy' 2>&1

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The git diff command should include error handling to prevent silent failures when the diff operation fails.

Suggested change
git diff -U0 HEAD~1 -- '*.groovy' 2>&1
git diff -U0 HEAD~1 -- '*.groovy' 2>&1 || return 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant